-
Notifications
You must be signed in to change notification settings - Fork 841
Created a new agent to showcase the list of components in the std catalog. Also fixed a few component bugs #596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ging on-the-wire a2UI JSON and actions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new component gallery sample, which is a valuable addition for demonstrating A2UI capabilities. The changes include a new agent, a Lit-based client, and improvements to existing UI components, notably a much-improved MultipleChoice component. While this is a great feature, there are several areas for improvement. The new code appears to have been partially copied from another sample, resulting in confusing naming and some irrelevant files that should be cleaned up. The new agent code also has some Python style guide violations. Finally, this large addition of functionality lacks corresponding tests for the agent logic, which is something to address per the repository's contribution guidelines.
|
Will be overhauling docs for this and other changes in a followup PR |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
… into md-componentslistagent
jacobsimionato
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we can tweak the structure of this to try to aim for consistency and reduce the overall side of the code we are maintaining.
There are already a couple of examples of galleries:
-
Catalog gallery at
samples/client/angular/projects/gallery, see https://github.com/google/A2UI/blob/main/samples/client/angular/projects/gallery/src/app/features/library/library.component.ts -
AG-UI's composer has a gallery: #365, https://a2ui-composer.ag-ui.com/components
I do think it'd be nice to have a gallery for each renderer, so we can compare how they render. I believe the other two galleries use the React and Angular renderers, so there is value in creating a Lit sample. How about trying to follow the structure of the other apps, to aim for consistency? This might involve:
-
Building this as a client-only app rather than an agent + client. I'm not sure if the agent adds much value if it just outputs canned content.
-
Reusing the same sample content as the other gallery apps. I think AG-UI folk provided nice sample content. Maybe we can reuse that in the other galleries? Then it's easy to compare between them.
I think there is value in having the agent as it is an e2e loop that gets tests - this is how most customers would use it too. In the sample, clicking on the log value button, the action is sent up to the agent and the agent updates a second surface. |
gspencergoog
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
I will check this in given the urgency and there are minimal renderer changes (bug fixes only). Will do any changes in followup as reqd |
Chatted offline. We will have any followup request from dit addressed later.
ditman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small comments in general. The changes to the web renderer seem all right. The new example app is rather large, but if you like it, let's go with it.
Note that there's a bunch of missing copyright notices at the top of the python code that should be there.
| return Array.isArray(this.selections) ? this.selections : []; | ||
| } | ||
| if (Array.isArray(this.selections)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the Array.isArray(this.selections) check duplicated?
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the extra whitespace?
(PS: We probably need autoformatting rules in the repo)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this file needa copyright header, like the a2ui schema?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Same comment about the copyright header)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Missing copyright header)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copyright header missing.
Also it might be nice to have an example of a silly tool for demonstration purposes (or remove the wiring for the empty file for clarity of the sample?)
|
|
||
| ## Running | ||
|
|
||
| This sample depends on the Lit renderer. Before running this sample, you need to build the renderer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add a script(s) to run the sample in the samples/lit/package.json?
…alog. Also fixed a few component bugs (google#596) * fix contact sample * Fix dropdown and make it multi-choice * Add a new component_gallery sample agent and client that allows debugging on-the-wire a2UI JSON and actions * Update samples/client/lit/component_gallery/README.md Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * fix modal * Address review comments --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>


Description
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
List which issues are fixed by this PR. For larger changes, raising an issue first helps reduce redundant work.
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.